Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

standards-compliant import.meta.resolve #10985

Merged
merged 7 commits into from
Feb 13, 2025
Merged

Conversation

turadg
Copy link
Member

@turadg turadg commented Feb 11, 2025

closes: #10986

Description

import.meta.resolve is synchronous now

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Kink to sort out with dapp integration test:

➤ YN0071: Cannot link @agoric/cosmic-swingset into @agoric/documentation@workspace:. dependency import-meta-resolve@npm:4.1.0 conflicts with parent dependency import-meta-resolve@npm:2.2.2
➤ YN0071: Cannot link @agoric/solo into @agoric/documentation@workspace:. dependency import-meta-resolve@npm:4.1.0 conflicts with parent dependency import-meta-resolve@npm:2.2.2
➤ YN0071: Cannot link @agoric/vats into @agoric/documentation@workspace:. dependency import-meta-resolve@npm:4.1.0 conflicts with parent dependency import-meta-resolve@npm:2.2.2

Upgrade Considerations

Copy link

cloudflare-workers-and-pages bot commented Feb 11, 2025

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: c5f7c97
Status: ✅  Deploy successful!
Preview URL: https://2ceb1ca2.agoric-sdk.pages.dev
Branch Preview URL: https://ta-import-meta-resolve.agoric-sdk.pages.dev

View logs

@turadg turadg force-pushed the ta/import-meta-resolve branch from 283ab20 to 989eec8 Compare February 11, 2025 21:11
@turadg turadg added the force:integration Force integration tests to run on PR label Feb 11, 2025
@turadg turadg force-pushed the ta/import-meta-resolve branch 4 times, most recently from 9660aa6 to ce9ff09 Compare February 12, 2025 20:59
@turadg turadg requested review from kriskowal and dckc February 12, 2025 20:59
@turadg turadg marked this pull request as ready for review February 12, 2025 21:00
@turadg turadg requested a review from a team as a code owner February 12, 2025 21:00
Copy link
Member

@kriskowal kriskowal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like progress. Thanks.

const importSpec = spec =>
importMetaResolve(spec, import.meta.url).then(u => new URL(u).pathname);
const importSpec = async spec =>
new URL(importMetaResolve(spec, import.meta.url)).pathname;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary to fix everything in one pass, but notably, this is an opportunity to replace .pathname with fileURLToPath as that’s more robust across platforms or checkout directories with spaces (.pathname will have %20 instead of the obligate space).

@turadg turadg force-pushed the ta/import-meta-resolve branch from ce9ff09 to 42b3213 Compare February 13, 2025 00:34
@turadg turadg force-pushed the ta/import-meta-resolve branch from 42b3213 to c5f7c97 Compare February 13, 2025 15:34
@turadg turadg added the automerge:rebase Automatically rebase updates, then merge label Feb 13, 2025
@mergify mergify bot merged commit e393fda into master Feb 13, 2025
92 of 93 checks passed
@mergify mergify bot deleted the ta/import-meta-resolve branch February 13, 2025 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge force:integration Force integration tests to run on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

synchronous import.meta.resolve
2 participants